refactor: hoist all devDeps to root #3032

overbalance · 2025-09-10T04:31:12Z

What this does

Hoists all build devDependencies to the root package.json.

14 fewer packages in node_modules (1.13% reduction)
878 fewer in dependency tree (11.39% reduction)
304 fewer packages added by npm ci (10.44% reduction)

Key changes

Test fixes

Updated karma configs to load plugins from root after dependency hoisting
Fixed webpack process polyfill path for ESM compatibility
Updated expect import syntax for socket.io tests
Updated AWS SDK mock response Content-Length headers

Dependencies

All build devDependencies moved to root package.json
Removed unused jQuery dependencies

Configuration

Converted root .mocharc.yml to .mocharc.json
Fixed axios import syntax in express example (namespace import → default import)

Package-specific changes

@opentelemetry/instrumentation-socket.io

Changed import * as expect to import expect (expect v29 uses default export)

@opentelemetry/instrumentation-dns

Added @ts-expect-error annotations for shimmer __wrapped properties

@opentelemetry/instrumentation-aws-sdk

Updated Bedrock mock response Content-Length headers

Browser packages (propagator-aws-xray, propagator-instana, instrumentation-user-interaction, instrumentation-long-task)

Updated karma.conf.js to load plugins from root package.json

trentm

Early feedback. I think this is looking pretty good. ;)

I don't know the karma code well.
I don't have a feel for the impact of the moduleResolution change in tsconfig.json files.
(not your fault) Eww on the assert module (because it shadows the core node.js module). Granted that was already installed at the top-level so this PR doesn't impact that at all.

Some questions/notes inline.

trentm · 2025-10-07T00:04:26Z

packages/instrumentation-openai/package.json

-    "expect": "29.2.0",
-    "nock": "^14.0.0",
-    "nyc": "17.1.0",
+    "form-data-encoder": "^4.1.0",


Is this one needed?

The openai dep brings in this package, though a MUCH older one.

[email protected] /Users/trentm/src/opentelemetry-js-contrib └─┬ @opentelemetry/[email protected] -> ./packages/instrumentation-openai └─┬ [email protected] └── [email protected]

I previously saw issues when running TAV but I just tried again and it's passing. I'll remove.

Okay, noted. I'll take a look. I know TAV well and I'm the component-owner of instr-openai.

packages/instrumentation-openai/tsconfig.json

trentm · 2025-10-07T00:07:56Z

packages/instrumentation-redis/package.json

  "peerDependencies": {
-    "@opentelemetry/api": "^1.3.0"
+    "@opentelemetry/api": "^1.3.0",
+    "redis": "^5.6.0"


Are we sure we want a change in peer deps here?

I'm not super familiar with how TAV works but when it installs different versions of redis, it only passes if no single version is specified in dependencies. I would like the owner to review if possible.

trentm · 2025-10-07T00:13:07Z

packages/resource-detector-gcp/.mocharc.json

Curious why this is the odd package out in needing this.

In cjs projects, mocha isn't able to process .mjs files without this config.
GCP has this unusual fixture:
argv: ['fixtures/detect-with-http-instrumentation.mjs'],

mocha shouldn't need to process that file, because it is a "fixture": There is a *.test.ts file in this package the exec's that fixture in a subprocess. Similar thing in a number of other packages' test suites.

Ok. Let me run it without the config and I'll post the error here.

Working now. Prob another artifact from TS 5.9

trentm · 2025-10-07T00:14:52Z

tsconfig.base.esm.json

  "compilerOptions": {
    "module": "es2022",
-    "moduleResolution": "node16"
+    "moduleResolution": "bundler"


I don't have a good sense of the potential impact of this. (https://www.typescriptlang.org/tsconfig/#moduleResolution)

Docs: https://www.typescriptlang.org/docs/handbook/modules/reference.html#packagejson-exports

The main point to be aware of is that you may only import files explicitly exported by the author. The "exports" key is read first when present in imported packages, and prevents paths like this:

import Something from "@swc/core/dist/esm/some-internal-folder/types/internal.js"

@overbalance We discussed this with other maintainers. Would it be straightforward to take the tsconfig changes out of this PR and do them in a separate PR? That would allow this PR to just be about "hoist all devDeps to root" and we could discuss the other change without all the noise of the hoisting.

Yep, reverted.

...s-sdk/test/mock-responses/bedrock-invokemodel-adds-amazon-nova-model-attributes-to-span.json

overbalance · 2025-10-10T14:56:44Z

@trentm If Node 18 support is needed then there's more work to do. I missed this requirement when I started this work. 😕 How do you want to proceed? Try to roll back dependency versions until it's happy?

❯ npm ci
npm warn EBADENGINE Unsupported engine {
npm warn EBADENGINE   package: '@azure-rest/[email protected]',
npm warn EBADENGINE   required: { node: '>=20.0.0' },
npm warn EBADENGINE   current: { node: 'v18.20.8', npm: '10.8.2' }
npm warn EBADENGINE }
npm warn EBADENGINE Unsupported engine {
npm warn EBADENGINE   package: '@azure/[email protected]',
npm warn EBADENGINE   required: { node: '>=20.0.0' },
npm warn EBADENGINE   current: { node: 'v18.20.8', npm: '10.8.2' }
npm warn EBADENGINE }

JamieDanielson · 2025-10-14T16:23:28Z

@trentm If Node 18 support is needed then there's more work to do. I missed this requirement when I started this work. 😕 How do you want to proceed? Try to roll back dependency versions until it's happy?

🤔 We say we support Active and Maintenance LTS versions of Node, and Node 18 is officially end of life. So it might be fine in Contrib to drop support for 18. That's a bigger discussion though (cc @open-telemetry/javascript-maintainers ) and we'll need to update docs too.

trentm · 2025-10-15T16:51:09Z

If Node 18 support is needed then there's more work to do. I missed this requirement when I started this work. 😕 How do you want to proceed? Try to roll back dependency versions until it's happy?
❯ npm ci
npm warn EBADENGINE Unsupported engine {

@overbalance I don't think those EBADENGINE warnings are blocking for this PR. We already have a number of those warnings in the current state. Unfortunately an EBADENGINE warning will matter for some deps, but not for some deps that are just for testing. For example this one:

npm warn EBADENGINE Unsupported engine {
npm warn EBADENGINE   package: '@azure/[email protected]',
npm warn EBADENGINE   required: { node: '>=20.0.0' },
npm warn EBADENGINE   current: { node: 'v18.20.8', npm: '10.8.2' }
npm warn EBADENGINE }

That dep is used by tedious@17:

% npm ls @azure/core-auth
[email protected] /Users/trentm/src/opentelemetry-js-contrib
└─┬ @opentelemetry/[email protected] -> ./packages/instrumentation-tedious
  └─┬ [email protected]
    ├─┬ @azure/[email protected]
    │ ├── @azure/[email protected]
    │ ├─┬ @azure/[email protected]
    │ │ └── @azure/[email protected] deduped
    │ └─┬ @azure/[email protected]
    │   └── @azure/[email protected] deduped
    └─┬ @azure/[email protected]
      ├─┬ @azure-rest/[email protected]
      │ └── @azure/[email protected] deduped
      ├── @azure/[email protected] deduped
      └─┬ @azure/[email protected]
        └── @azure/[email protected] deduped

which is a test dep. We intentionally install a recent-ish major version of tedious so that unit tests (i.e. npm test) for an instrumentation (instrumentation-tedious in this case) tests with a relatively modern version of the target package. We then rely on our test-all-versions tests to handle testing against those versions of tedious that work with older Node.js versions (e.g. Node.js v18)

overbalance · 2025-10-17T19:57:32Z

@trentm Those warnings are blocking CI from what I see. What do you recommend?

trentm · 2025-10-17T23:19:57Z

Those warnings are blocking CI from what I see.

I don't know what is going on. I can npm ci this feature branch locally.

This CI step failed with a network error: https://github.com/open-telemetry/opentelemetry-js-contrib/actions/runs/18537223726/job/53057847496?pr=3032

...
npm WARN EBADENGINE }
npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE   package: '[email protected]',
npm WARN EBADENGINE   required: { node: '20 || >=22' },
npm WARN EBADENGINE   current: { node: 'v18.19.0', npm: '10.2.3' }
npm WARN EBADENGINE }
npm ERR! code ECONNRESET
npm ERR! network aborted
npm ERR! network This is a problem related to network connectivity.
npm ERR! network In most cases you are behind a proxy or have bad network settings.
npm ERR! network 
npm ERR! network If you are behind a proxy, please make sure that the
npm ERR! network 'proxy' config is set properly.  See: 'npm help config'

npm ERR! A complete log of this run can be found in: /home/runner/.npm/_logs/2025-10-17T23_04_44_742Z-debug-0.log
Error: Process completed with exit code 1.

The other test jobs are stuck in npm ci. I cancelled the run after 20m.
I think the "network aborted" above is just a fluke. The real issue is the stuck npm ci steps.

The lint workflow, which does npm ci --ignore-scripts, worked fine.

Ideas on what this could be:

Some install/build/postinstall script in one of the deps is breaking things.
Some GitHub Actions or npmjs bug/incident that is causing this to break.
Some limit is being hit in GH Actions? THis is total guess.

overbalance force-pushed the overbalance/hoist-all-shared-deps branch from f104904 to a80544a Compare September 26, 2025 21:32

overbalance changed the title ~~build: upgrade TypeScript to 5.9.2, fix CommonJS module resolution~~ build: hoist all devDeps to root, fix CommonJS module resolution Sep 26, 2025

overbalance marked this pull request as ready for review September 27, 2025 16:41

overbalance requested a review from a team as a code owner September 27, 2025 16:41

overbalance force-pushed the overbalance/hoist-all-shared-deps branch 3 times, most recently from d186426 to 6e89df1 Compare October 6, 2025 23:36

trentm added the has:sponsor This package or feature has a sponsor that has volunteered to review PRs and respond to questions label Oct 6, 2025

trentm reviewed Oct 7, 2025

View reviewed changes

overbalance force-pushed the overbalance/hoist-all-shared-deps branch 5 times, most recently from 4797803 to 723401a Compare October 8, 2025 14:40

overbalance requested a review from trentm October 8, 2025 14:48

overbalance force-pushed the overbalance/hoist-all-shared-deps branch from 723401a to 3056842 Compare October 10, 2025 06:05

overbalance changed the title ~~build: hoist all devDeps to root, fix CommonJS module resolution~~ build: hoist all devDeps to root Oct 10, 2025

overbalance force-pushed the overbalance/hoist-all-shared-deps branch from 3056842 to 804aa71 Compare October 13, 2025 16:19

overbalance added 4 commits October 15, 2025 12:24

refactor(devDeps): hoist devDeps

a84ece8

openai: revert skipLibCheck

cd19966

consistency

53181d2

fix merge

6ac6dbd

overbalance force-pushed the overbalance/hoist-all-shared-deps branch from 46672a0 to 6ac6dbd Compare October 15, 2025 17:24

overbalance changed the title ~~build: hoist all devDeps to root~~ refactor: hoist all devDeps to root Oct 15, 2025

trentm mentioned this pull request Oct 17, 2025

chore: [DO NOT MERGE] debugging CI failures in PR 3032 #3181

Draft

refactor: hoist all devDeps to root #3032

Are you sure you want to change the base?

refactor: hoist all devDeps to root #3032

Uh oh!

Conversation

overbalance commented Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this does

Key changes

Test fixes

Dependencies

Configuration

Package-specific changes

@opentelemetry/instrumentation-socket.io

@opentelemetry/instrumentation-dns

@opentelemetry/instrumentation-aws-sdk

Browser packages (propagator-aws-xray, propagator-instana, instrumentation-user-interaction, instrumentation-long-task)

Uh oh!

trentm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

overbalance Oct 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

overbalance Oct 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

overbalance Oct 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

overbalance commented Oct 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JamieDanielson commented Oct 14, 2025

Uh oh!

trentm commented Oct 15, 2025

Uh oh!

overbalance commented Oct 17, 2025

Uh oh!

trentm commented Oct 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

13 participants

overbalance commented Sep 10, 2025 •

edited

Loading

overbalance Oct 8, 2025 •

edited

Loading

overbalance Oct 8, 2025 •

edited

Loading

overbalance Oct 8, 2025 •

edited

Loading

overbalance commented Oct 10, 2025 •

edited

Loading

trentm commented Oct 17, 2025 •

edited

Loading